-
Notifications
You must be signed in to change notification settings - Fork 41
More explicit handling for end-of-group bit #1118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The previous "implicit" end of group handling both created additional malformed track cases, and was challenging to integrate into existing MoQ APIs. Change the interpretation to mean it's the same as receiving an End of Group Object with ID + 1. Fixes: #1093
fluffy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with this but it seems like a really weird way to specify what happens. I'd rather just see it say what happens when you get a UDP packet like than put it in terms of a UDP packet but don't really care.
martinduke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implies that if I want extensions for my EndOfGroup object, I need to send that object explicitly. That's fine, I suppose, but it is a little different from when a single object had payload, extensions, and EoG status.
I don't understand many of the use cases for extensions, so I don't know if this is a problem.
This gets thorny if the relay decides to add an extension to an EoG object. If it's concerned it might do this, it can never initiate a stream with the EndOfGroup flag.
I am not sure compressing away the EndOfGroup object is well-conceived. It's already opened up a series of logical problems for what is a minor savings.
Yes, this is correct.
I can conceive of use cases for extensions in EOG -- metrics covering the entire group or something. For a relay that might add extensions to EOG objects, it should be clear it can't use this flag.
It was not well conceived in the first instance as revealed by implementation experience. I was hoping this fixed it, but as you point out, it's a little fuzzy around extension handling. Do you think we should remove the feature or try to fix-forward? |
|
What if Object ID is VARINT62_MAX? I can't express the implied EOG object in a FETCH stream. |
Wait for #1016 ? |
That just moves the problem to a different integer. |
|
Having now implemented the existing spec (not this PR), and realized that this is saving 3 bytes per group under a narrow set of applicability conditions, I believe that all of this superstructure is not worth the lines of spec, lines of code, and ~dozen test cases it spawns. I propose we revert the whole thing. |
|
For me, low bandwidth audio is the case that drives this being worth doing. |
|
#1342 will address this in a more complete way |
The previous "implicit" end of group handling both created additional malformed track cases, and was challenging to integrate into existing MoQ APIs. Change the interpretation to mean it's the same as receiving an End of Group Object with ID + 1.
Fixes: #1093